CSHARP-5777: Avoid ThreadPool-dependent IO methods in sync API #1813
Conversation
sanych-sun
added
the
chore
Consolidate Stream extension methods.
sanych-sun
force-pushed
the
csharp5777
branch
from
b4aa35b to
b2bf468
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR refactors stream extension methods to consolidate implementations and avoid ThreadPool-dependent I/O methods in the synchronous API. The main changes involve replacing the old API that used OperationContext and TimeSpan parameters with a cleaner API that uses int timeoutMs and CancellationToken parameters directly.
Key Changes:
- Consolidated stream extension methods with separate timeout handling for sync and async operations
- Replaced
TimeSpantimeout parameters withint timeoutMs(milliseconds) for consistency - Improved test coverage by adding dedicated tests for byte array overloads of WriteBytes methods
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/MongoDB.Driver/Core/Misc/StreamExtensionMethods.cs | Complete refactoring of ReadBytes/WriteBytes extension methods with new timeout handling mechanism; sync methods now use Timer-based approach while async methods use Task.WhenAny pattern |
| tests/MongoDB.Driver.Tests/Core/Misc/StreamExtensionMethodsTests.cs | Updated all tests to use new API signatures; added comprehensive test coverage for byte array WriteBytes methods; renamed tests for better clarity |
| src/MongoDB.Driver/GridFS/GridFSBucket.cs | Updated ReadBytes/ReadBytesAsync calls to use new API with named cancellationToken parameter |
| src/MongoDB.Driver/Core/Connections/BinaryConnection.cs | Updated ReadBytes/WriteBytes calls with TimeSpan-to-milliseconds conversion at call sites |
| src/MongoDB.Driver/Core/Connections/Socks5Helper.cs | Updated ReadBytes/ReadBytesAsync calls to use new API with named cancellationToken parameter |
| src/MongoDB.Driver/Core/Compression/SnappyCompressor.cs | Simplified ReadBytes calls by removing unnecessary timeout parameters for in-memory operations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/MongoDB.Driver.Tests/Core/Misc/StreamExtensionMethodsTests.cs
Outdated
Show resolved
Hide resolved
| (str, state) => | ||
| { | ||
| var bytesRead = 0; | ||
| while (bytesRead < state.count) |
There was a problem hiding this comment.
nit: Probably not relevant here, but comparing to 0 is a bit faster, so no reason to change previous code that used remainingCount variable. For your consideration.
There was a problem hiding this comment.
Thank you for spotting this. Now should be done.
BorisDog
left a comment
BorisDog
left a comment
There was a problem hiding this comment.
Just few minor comments.
| (str, state) => | ||
| { | ||
| var bytesRead = 0; | ||
| while (bytesRead < state.count) |
2 participants
Consolidate Stream extension methods + improve code coverage.
Includes fix for Tailable_cursor_should_be_able_to_be_cancelled_from_a_different_thread_with_expected_result flaky test.